Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DPG best-practice and automation template #28016

Merged
merged 30 commits into from
May 7, 2022
Merged

Conversation

chunyu3
Copy link
Member

@chunyu3 chunyu3 commented Apr 6, 2022

Contributing to the Azure SDK

Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.

For specific information about pull request etiquette and best practices, see this section.

As part of the DPG effort, we are providing a DPG template as a tool and guidance to our service team partners:

  1. To enable them to create a new SDK in the repo for their service via this template, that builds and passes CIs in 10 minutes
    That has all the information they need to follow Azure SDK best practices to create tests, samples, and other customer-facing documents in a matter of days
  2. This template sdk/template/Azure.Template is DPG + grow-up story, and it can pass the CI mock test to validate the changes of eng scripts and pipeline.

@ghost ghost added the EngSys This issue is impacting the engineering system. label Apr 6, 2022
@chunyu3 chunyu3 marked this pull request as draft April 6, 2022 08:48
@azure-sdk
Copy link
Collaborator

API change check for Azure.Template

API changes have been detected in Azure.Template. You can review API changes here

Copy link
Member

@annelo-msft annelo-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a link to the issue in the PR description.

sdk/template/Azure.Template/CHANGELOG.md Outdated Show resolved Hide resolved
sdk/template/Azure.Template/README.md Outdated Show resolved Hide resolved
sdk/template/Azure.Template/src/Azure.Template.csproj Outdated Show resolved Hide resolved
sdk/template/Azure.Template/src/GlobalSuppressions.cs Outdated Show resolved Hide resolved
sdk/template/Azure.Template/src/GlobalSuppressions.cs Outdated Show resolved Hide resolved
sdk/template/Azure.Template/src/TemplateClient.cs Outdated Show resolved Hide resolved
sdk/template/ci.yml Outdated Show resolved Hide resolved
@weshaggard
Copy link
Member

For the content\ci.yml files you might need to rename them so that our automation that sets up pipelines doesn't try to setup a pipeline for them. Perhaps we can name them ci.yml.template or something like that and as part of the template generation process rename the file to ci.yml.

@weshaggard
Copy link
Member

FYI @ArthurMa1978 given that this change impacts the mgmt template as well.

@heaths
Copy link
Member

heaths commented Apr 6, 2022

  1. Tip: for a PR, instead of merging from main which can make PRs difficult to review in some scenarios, you can more cleanly rebase on main to merge e.g.,
    git fetch upstream main
    git rebase upstream/main # run this in your topic branch
    git push --force
  2. Given the email thread, perhaps we want to consider keeping eng/template the only templates and have that generate sdk/sample/Azure.Template.Sample or maybe sdk/template/Azure.Template.Sample and remove the .template.config directory from sdk/template now.

@lirenhe lirenhe added the DPG label Apr 7, 2022
@check-enforcer
Copy link

This pull request is protected by Check Enforcer.
For more information about how to run a pipeline against this pull request, see this.

@chunyu3 chunyu3 force-pushed the codeTemplate branch 2 times, most recently from 9569995 to aabcdb8 Compare April 12, 2022 08:52
Copy link
Member

@heaths heaths left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest converting this to a full PR instead of a draft. It'll run more checks that would've cause various syntax errors I noticed. We typically only use draft PRs when you basically want to try out some idea and make sure it's obvious it shouldn't be merged yet, even if reviewed and approved.

@chunyu3 chunyu3 force-pushed the codeTemplate branch 3 times, most recently from 876aa27 to 9a5c884 Compare April 18, 2022 02:18
Copy link
Member

@heaths heaths left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I install the template from your PR and create a new project, I still see a number of issues:

  1. Lowercase project name, which will impact the output file name and other issues:
    image
    image
  2. The README and CHANGELOG.md files were in the current directory. You need to create a subdirectory for the calculated Package name. For example, if I run dotnet new azsdkdpg -g Test -li Sample -s local.json in sdk/test then I should see a number of files in sdk/test but then also an Azure.Test.Sample directory that has the src, tests, etc. directories.
  3. If I search sdk/test (from my example above) for "template", I still find a lot of links with "template" in the URL:
    image
    If you checkout main and create a new project with the old sdk/template these all get cleaned up. Take a look at all the replacements I do:
    "ProjectNameUnderscored": {
    "type": "generated",
    "generator": "regex",
    "parameters": {
    "source": "ProjectName",
    "steps": [
    {
    "regex": "\\.",
    "replacement": "_"
    }
    ]
    },
    "replaces": "Azure_Template"
    },
    "ServiceDirectoryPath": {
    "type": "generated",
    "generator": "join",
    "parameters": {
    "symbols": [
    {
    "type": "const",
    "value": "sdk/"
    },
    {
    "type": "ref",
    "value": "ServiceDirectory"
    }
    ],
    "separator": ""
    },
    "replaces": "sdk/template"
    },
    "SafeProjectName": {
    "type": "generated",
    "generator": "regex",
    "parameters": {
    "source": "ProjectName",
    "steps": [
    {
    "regex": "\\.",
    "replacement": ""
    }
    ]
    },
    "replaces": "AzureTemplate"
    },
    "YamlServiceDirectory": {
    "type": "generated",
    "generator": "join",
    "parameters": {
    "symbols": [
    {
    "type": "const",
    "value": "ServiceDirectory: "
    },
    {
    "type": "ref",
    "value": "ServiceDirectory"
    }
    ],
    "separator": ""
    },
    "replaces": "ServiceDirectory: template"
    }

    Everywhere you see "replaces" I'm replacing patterns. You do a great job of adding placeholders, but those need to be used in more places or just replace "sdk/template" with "sdk/{ServiceDirectory}" where "ServiceDirectory" is one of the required inputs I take. In fact, I was able to calculate just about everything with only 2 inputs but you could use the separate GroupName and LibraryName you do now - just add a calculated field that combines "Azure.{GroupName}.{LibraryName" into one symbol you replace "Azure.Template", create another replacement with "Azure{GroupName}{LibraryName}" for "AzureTemplate" (the "safe" name, as I called it), etc.

I recommend using dotnet new azsdkdpg in a directory. What comes out should match the same structure we use for any track 2 SDK e.g.,

  • sdk/
    • {ServiceDirectory}/
      • Azure.{GroupName}.{LibraryName}/
        • src/
          • Azure.{GroupName}.{LibraryName}.csproj
          • (other files)
        • tests/
          • Azure.{GroupName}.{LibraryName}.Tests.csproj
          • (other files)
        • perf/ (and other directories, with proper project names)
        • Azure.{GroupName}.{LibraryName}.sln
        • README.md
        • CHANGELOG.md
      • Directory.Build.props
      • test-resources.json
      • ci.yml
      • tests.yml

@weshaggard
Copy link
Member

@chunyu3 to help us see what the final output is could you please run the template generation step and push the changes to another branch so we can see what it contains. Also list the exact generation command you used to generate it here.

@chunyu3
Copy link
Member Author

chunyu3 commented May 6, 2022

Hi @weshaggard @heaths I updated the dpg template according to our discussion in the meeting. And I generated two sample SDKs via azsdkdpg template. You can specify dotnet new --name option or not.

  1. under sdk/sample directory, specify the dotnet --name option, the value of name is Azure.<group>.<service>, run cmd dotnet new azsdkdpg --name Azure.IoT.MyService --groupName IoT --clientName MyService --serviceDirectory test --swagger https://github.com/dpokluda/azure-rest-api-specs/blob/be397aa65510bd4e8f87da539af2b0025f6f44ca/specification/deviceupdate/data-plane/Microsoft.DeviceUpdate/preview/2020-09-01/deviceupdate.json --securityScopes https://api.adu.microsoft.com/.default

The generated SDK is https://github.com/chunyu3/azure-sdk-for-net/tree/TemplateSample/sdk/sample

  1. under sdk/test directory, don't specify the dotnet --name option, it will use the test as the package directory name. run cmd dotnet new azsdkdpg --groupName IoT --clientName MyService --serviceDirectory test --swagger https://github.com/dpokluda/azure-rest-api-specs/blob/be397aa65510bd4e8f87da539af2b0025f6f44ca/specification/deviceupdate/data-plane/Microsoft.DeviceUpdate/preview/2020-09-01/deviceupdate.json --securityScopes https://api.adu.microsoft.com/.default

The generated SDK is https://github.com/chunyu3/azure-sdk-for-net/tree/TemplateSample/sdk/test

@chunyu3
Copy link
Member Author

chunyu3 commented May 6, 2022

Also, we can run Invoke-DataPlaneGenerateSDKPackage.ps1 to generate the whole SDK starter package.

pwsh eng/scripts/automation/Invoke-DataPlaneGenerateSDKPackage.ps1 -service test -namespace Azure.Identity.MySample -sdkPath ./azure-sdk-for-net -inputfiles https://github.com/dpokluda/azure-rest-api-specs/blob/be397aa65510bd4e8f87da539af2b0025f6f44ca/specification/deviceupdate/data-plane/Microsoft.DeviceUpdate/preview/2020-09-01/deviceupdate.json -securityScope https://myservice/.default

@chunyu3 chunyu3 requested a review from weshaggard May 6, 2022 05:53
@chunyu3 chunyu3 merged commit c727827 into Azure:main May 7, 2022
@chunyu3 chunyu3 deleted the codeTemplate branch June 6, 2022 08:51
paterasMSFT pushed a commit that referenced this pull request Jun 15, 2022
* Add request condition argument check

* resolve build failure

* remove unused import

* remove unused Azure.Core dependency

* move eng template to sdk

* update sample template to DPG + growup

* update test cases

* remove the templates from eng directory

* update Azure.ServiceTemplate.Template

* merge Azure.ServiceTemplate.Template with Azure.Template

* update api

* resolve rebase conflict

* complete Azure.Template to match DPG guideline

* refine template samples

* update template content

* remove project file in template content

* refine snippet in Readme

* resolve inner scope conflict issule

* refine test

* remove duplicate snippet

* update template sample

* update script to use the Azure.Template

* add function to update ci.yml

* remove log in script

* resolve comments

* use actual type instead of var

* resolve comments

* refine readme files, add error checking in outer script

* add Azure.Template as the content of template

* move .content folder out of Azure.Template
zhihaoxue pushed a commit to zhihaoxue/azure-sdk-for-net that referenced this pull request Jul 27, 2022
* Add request condition argument check

* resolve build failure

* remove unused import

* remove unused Azure.Core dependency

* move eng template to sdk

* update sample template to DPG + growup

* update test cases

* remove the templates from eng directory

* update Azure.ServiceTemplate.Template

* merge Azure.ServiceTemplate.Template with Azure.Template

* update api

* resolve rebase conflict

* complete Azure.Template to match DPG guideline

* refine template samples

* update template content

* remove project file in template content

* refine snippet in Readme

* resolve inner scope conflict issule

* refine test

* remove duplicate snippet

* update template sample

* update script to use the Azure.Template

* add function to update ci.yml

* remove log in script

* resolve comments

* use actual type instead of var

* resolve comments

* refine readme files, add error checking in outer script

* add Azure.Template as the content of template

* move .content folder out of Azure.Template
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DPG EngSys This issue is impacting the engineering system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants